feat: add options to restrict certain namespaces to certain login methods#2876
Open
JivusAyrus wants to merge 33 commits into
Open
feat: add options to restrict certain namespaces to certain login methods#2876JivusAyrus wants to merge 33 commits into
JivusAyrus wants to merge 33 commits into
Conversation
…45-controlplane-different-auth-providers-for-different
JivusAyrus
requested review from
a team,
Aenimus,
StarpTech,
endigma,
thisisnithin and
wilsonrivera
as code owners
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughImplements IdP-aware namespace access gating: adds LoginMethod/OIDC provider and namespace-SSO proto types and RPCs, DB schema and repositories for namespace SSO mappings, resolves login method from identity_provider claims, computes per-session allowed namespaces, enforces an IdP allowlist in RBAC and queries, and adds admin UI for mappings. ChangesMulti-Provider SSO Namespace Access Gating
🎯 4 (Complex) | ⏱️ ~75 minutes
|
Router-nonroot image scan passed✅ No security vulnerabilities found in image: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
controlplane/src/core/bufservices/user/updateOrgMemberGroup.ts (1)
63-88:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeycloak client authentication happens after the SSO user checks.
The loop at lines 68-85 calls
opts.keycloakClient.client.users.find()for each provider, butopts.keycloakClient.authenticateClient()is only called at line 88, after the loop. This may cause the Keycloak calls inside the loop to fail if the client token isn't already valid.Consider moving the authentication call before the loop:
🔧 Proposed fix
// Ensure that the organization member has not signed in with SSO via any // of the org's configured providers. + await opts.keycloakClient.authenticateClient(); + const providers = await oidcRepo.listOidcProvidersByOrganizationId({ organizationId: authContext.organizationId, }); for (const provider of providers) { // checking if the user has logged in using the sso const ssoUser = await opts.keycloakClient.client.users.find({ realm: opts.keycloakRealm, email: orgMember.email, exact: true, idpAlias: provider.alias, }); if (ssoUser.length > 0) { return { response: { code: EnumStatusCode.ERR, details: 'User has logged in using the OIDC provider. Please update the group using the provider.', }, }; } } // Retrieve the Keycloak user - await opts.keycloakClient.authenticateClient(); const users = await opts.keycloakClient.client.users.find({🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controlplane/src/core/bufservices/user/updateOrgMemberGroup.ts` around lines 63 - 88, The Keycloak client is authenticated after you call opts.keycloakClient.client.users.find() inside the providers loop (after listOidcProvidersByOrganizationId), which can cause those calls to fail; fix it by calling and awaiting opts.keycloakClient.authenticateClient() before entering the for (const provider of providers) loop so the client has a valid token, and keep the existing find calls (client.users.find) unchanged; also propagate or handle any authentication errors from authenticateClient() before proceeding.
🧹 Nitpick comments (2)
controlplane/src/core/services/RBACEvaluator.ts (1)
115-117: 💤 Low valueMinor: Redundant IdP gate check in
canCreateContract.
canCreateFederatedGraphalready callsisAllowedByIdpGate, so the additional check on line 116 is redundant. This doesn't cause incorrect behavior but adds unnecessary computation.♻️ Suggested simplification
canCreateContract(namespace: Namespace) { - return this.canCreateFederatedGraph(namespace) && this.isAllowedByIdpGate(namespace.id); + return this.canCreateFederatedGraph(namespace); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controlplane/src/core/services/RBACEvaluator.ts` around lines 115 - 117, canCreateContract currently performs an extra isAllowedByIdpGate check despite canCreateFederatedGraph already invoking isAllowedByIdpGate; remove the redundant check from canCreateContract so it simply returns the result of canCreateFederatedGraph(namespace) to avoid duplicate computation while preserving behavior (refer to the canCreateContract, canCreateFederatedGraph, and isAllowedByIdpGate symbols and the Namespace parameter).controlplane/src/core/bufservices/organization/whoAmI.ts (1)
4-9: 💤 Low valueStatic analysis false positive for generated protobuf types.
The linter flags
LoginMethod_Typefor not being in camelCase, but this identifier is auto-generated from the protobuf definitions and cannot be renamed here. Consider adding a lint suppression comment if this causes CI failures, or configure the linter to ignore generated import identifiers.+// eslint-disable-next-line `@typescript-eslint/naming-convention` import { LoginMethod, LoginMethod_Type,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controlplane/src/core/bufservices/organization/whoAmI.ts` around lines 4 - 9, The import brings in auto-generated protobuf identifiers like LoginMethod_Type which trigger the linter's camelCase rule; add a local lint suppression so the generated name is allowed: place an ESLint/TSLint disable comment immediately above the import line (targeting the naming-convention/camelcase rule) or add a one-line disable for the specific rule, referencing the imported symbol LoginMethod_Type to make it clear why the rule is disabled; ensure the comment is minimal and scoped (not file-wide) so only the generated import is exempted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@controlplane/src/core/bufservices/sso/deleteOIDCProvider.ts`:
- Around line 46-53: Move the request validation for req.id to run before any
Keycloak client authentication/initialization so invalid input returns
ERR_BAD_REQUEST deterministically; specifically, in deleteOIDCProvider (and
before any calls to getKeycloakClient / keycloakAdminClient.authenticate or
usage of variables like keycloakClient/keycloakAdminClient), check if (!req.id)
and return the ERR_BAD_REQUEST response immediately, then proceed to
create/authenticate the Keycloak client only after validation succeeds.
In `@controlplane/src/core/repositories/NamespaceSsoMappingRepository.ts`:
- Around line 70-81: The getMapping method is declared async but doesn't await
the database promise; remove the unnecessary async keyword from the getMapping
declaration (or alternatively add an await before
this.db.select(...).from(...).where(...).execute()) so the function either
returns the promise directly or properly awaits it; update the getMapping
signature and keep the existing query body referencing namespaceSsoProviders and
eq(namespaceSsoProviders.namespaceId, input.namespaceId).
In `@controlplane/src/core/repositories/OidcRepository.ts`:
- Around line 32-39: The method listOidcProvidersByOrganizationId is declared
async but does not use await; remove the unnecessary async modifier (or
alternatively await the DB promise) to fix the lint warning—update the method
signature in OidcRepository (and mirror the same fix pattern used in
NamespaceSsoMappingRepository) so it returns the DB query promise directly
without async, or add an await before this.db.select(...).execute() if you
prefer an awaited result.
In `@proto/wg/cosmo/platform/v1/platform.proto`:
- Around line 1896-1898: Update the GetOIDCProviderRequest (and analogous
messages at the other locations) to make the id field optional (i.e., mark it as
optional/allow empty) in the proto definition for message GetOIDCProviderRequest
and the similar messages at lines noted, then change the server handlers
GetOIDCProvider, DeleteOIDCProvider, and UpdateIDPMappers to tolerate a missing
id by implementing fallback resolution: if request.id is empty, try to infer the
provider id from the authenticated organization context (or from a single
default provider for that org) and only return a clear ERR_BAD_REQUEST when both
the id is missing and no unique provider can be resolved; ensure validation
error messages explicitly state when an id is required versus when a provider
could not be inferred.
- Around line 3100-3104: Change the UpdateNamespaceSSOMappingRequest proto so
allow_password_login is declared as "optional bool allow_password_login = 3;"
instead of plain bool, and then update the handler that processes
UpdateNamespaceSSOMappingRequest to check the presence of allow_password_login
(i.e., whether it is set/defined) before calling setMapping(); only pass a value
to setMapping() when the field is present (treat absent as “no change”), and
preserve existing behavior for explicit true/false inputs so setMapping()
continues to be called with explicit values when provided.
In `@studio/src/components/dashboard/namespace-selector.tsx`:
- Around line 91-100: The trigger hides the namespace label when
isViewingGraphOrSubgraph is true, which leaves only the caret visible even when
hasNoAccess is true; update the render logic in namespace-selector.tsx so that
when hasNoAccess is true you still render displayName (e.g., "No access" or the
existing displayName) even if isViewingGraphOrSubgraph is true—adjust the
conditional around the span that uses truncateNamespace, hasNoAccess and
displayName so it renders whenever hasNoAccess is true OR
isViewingGraphOrSubgraph is false, and ensure the className still applies
hasNoAccess styling.
In `@studio/src/components/user-menu.tsx`:
- Line 85: The SSO label currently uses loginMethod.ssoProviderName ||
loginMethod.ssoAlias || '' which can produce an empty string; update the logic
that sets ssoLabel (and the similar logic around lines 108-111) to provide a
fallback like 'SSO' instead of ''. For example, change the expression that
defines ssoLabel in component user-menu.tsx to use loginMethod.ssoProviderName
|| loginMethod.ssoAlias || 'SSO' (and apply the same fallback where the other
SSO display label is computed) so the UI never renders "Logged in via " with a
blank provider.
---
Outside diff comments:
In `@controlplane/src/core/bufservices/user/updateOrgMemberGroup.ts`:
- Around line 63-88: The Keycloak client is authenticated after you call
opts.keycloakClient.client.users.find() inside the providers loop (after
listOidcProvidersByOrganizationId), which can cause those calls to fail; fix it
by calling and awaiting opts.keycloakClient.authenticateClient() before entering
the for (const provider of providers) loop so the client has a valid token, and
keep the existing find calls (client.users.find) unchanged; also propagate or
handle any authentication errors from authenticateClient() before proceeding.
---
Nitpick comments:
In `@controlplane/src/core/bufservices/organization/whoAmI.ts`:
- Around line 4-9: The import brings in auto-generated protobuf identifiers like
LoginMethod_Type which trigger the linter's camelCase rule; add a local lint
suppression so the generated name is allowed: place an ESLint/TSLint disable
comment immediately above the import line (targeting the
naming-convention/camelcase rule) or add a one-line disable for the specific
rule, referencing the imported symbol LoginMethod_Type to make it clear why the
rule is disabled; ensure the comment is minimal and scoped (not file-wide) so
only the generated import is exempted.
In `@controlplane/src/core/services/RBACEvaluator.ts`:
- Around line 115-117: canCreateContract currently performs an extra
isAllowedByIdpGate check despite canCreateFederatedGraph already invoking
isAllowedByIdpGate; remove the redundant check from canCreateContract so it
simply returns the result of canCreateFederatedGraph(namespace) to avoid
duplicate computation while preserving behavior (refer to the canCreateContract,
canCreateFederatedGraph, and isAllowedByIdpGate symbols and the Namespace
parameter).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 300c6e7a-cd0b-4e45-bdb0-6e81f7d163be
⛔ Files ignored due to path filters (2)
connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.gois excluded by!**/*.pb.go,!**/gen/**connect-go/gen/proto/wg/cosmo/platform/v1/platformv1connect/platform.connect.gois excluded by!**/gen/**
📒 Files selected for processing (71)
connect/src/wg/cosmo/platform/v1/platform-PlatformService_connectquery.tsconnect/src/wg/cosmo/platform/v1/platform_connect.tsconnect/src/wg/cosmo/platform/v1/platform_pb.tscontrolplane/migrations/0139_nappy_fallen_one.sqlcontrolplane/migrations/meta/0139_snapshot.jsoncontrolplane/migrations/meta/_journal.jsoncontrolplane/src/core/auth-utils.tscontrolplane/src/core/bufservices/PlatformService.tscontrolplane/src/core/bufservices/analytics/getAnalyticsView.tscontrolplane/src/core/bufservices/analytics/getDashboardAnalyticsView.tscontrolplane/src/core/bufservices/analytics/getFieldUsage.tscontrolplane/src/core/bufservices/analytics/getGraphMetrics.tscontrolplane/src/core/bufservices/analytics/getMetricsErrorRate.tscontrolplane/src/core/bufservices/analytics/getOperationClients.tscontrolplane/src/core/bufservices/analytics/getOperationContent.tscontrolplane/src/core/bufservices/analytics/getOperationDeprecatedFields.tscontrolplane/src/core/bufservices/analytics/getOperations.tscontrolplane/src/core/bufservices/analytics/getTrace.tscontrolplane/src/core/bufservices/cache-warmer/computeCacheWarmerOperations.tscontrolplane/src/core/bufservices/cache-warmer/configureCacheWarmer.tscontrolplane/src/core/bufservices/feature-flag/getFeatureFlags.tscontrolplane/src/core/bufservices/federated-graph/getClientsFromAnalytics.tscontrolplane/src/core/bufservices/federated-graph/getCompositionDetails.tscontrolplane/src/core/bufservices/federated-graph/migrateFromApollo.tscontrolplane/src/core/bufservices/federated-graph/moveFederatedGraph.tscontrolplane/src/core/bufservices/namespace/getNamespaceSSOMapping.tscontrolplane/src/core/bufservices/namespace/updateNamespaceSSOMapping.tscontrolplane/src/core/bufservices/organization/deleteOrganizationGroup.tscontrolplane/src/core/bufservices/organization/getOrganizationGroups.tscontrolplane/src/core/bufservices/organization/whoAmI.tscontrolplane/src/core/bufservices/persisted-operation/getClients.tscontrolplane/src/core/bufservices/persisted-operation/getPersistedOperations.tscontrolplane/src/core/bufservices/proposal/getNamespaceProposalConfig.tscontrolplane/src/core/bufservices/proposal/getProposal.tscontrolplane/src/core/bufservices/proposal/getProposalChecks.tscontrolplane/src/core/bufservices/schema-version/getChangelogBySchemaVersion.tscontrolplane/src/core/bufservices/sso/createOIDCProvider.tscontrolplane/src/core/bufservices/sso/deleteOIDCProvider.tscontrolplane/src/core/bufservices/sso/getOIDCProvider.tscontrolplane/src/core/bufservices/sso/listOIDCProviders.tscontrolplane/src/core/bufservices/sso/updateIDPMappers.tscontrolplane/src/core/bufservices/user/updateOrgMemberGroup.tscontrolplane/src/core/build-server.tscontrolplane/src/core/controllers/auth.tscontrolplane/src/core/repositories/FeatureFlagRepository.tscontrolplane/src/core/repositories/FederatedGraphRepository.tscontrolplane/src/core/repositories/GraphCompositionRepository.tscontrolplane/src/core/repositories/NamespaceRepository.tscontrolplane/src/core/repositories/NamespaceSsoMappingRepository.tscontrolplane/src/core/repositories/OidcRepository.tscontrolplane/src/core/repositories/SubgraphRepository.tscontrolplane/src/core/repositories/UserRepository.tscontrolplane/src/core/services/AccessTokenAuthenticator.tscontrolplane/src/core/services/ApiKeyAuthenticator.tscontrolplane/src/core/services/Authentication.tscontrolplane/src/core/services/Authorization.tscontrolplane/src/core/services/RBACEvaluator.tscontrolplane/src/core/services/WebSessionAuthenticator.tscontrolplane/src/core/workers/DeleteOrganizationWorker.tscontrolplane/src/db/models.tscontrolplane/src/db/schema.tscontrolplane/src/types/index.tsdocker/keycloak/realm.jsonproto/wg/cosmo/platform/v1/platform.protostudio/src/components/app-provider.tsxstudio/src/components/dashboard/namespace-gate-empty-state.tsxstudio/src/components/dashboard/namespace-selector.tsxstudio/src/components/layout/dashboard-layout.tsxstudio/src/components/user-menu.tsxstudio/src/pages/[organizationSlug]/namespace-sso.tsxstudio/src/pages/[organizationSlug]/settings.tsx
💤 Files with no reviewable changes (1)
- controlplane/src/core/bufservices/sso/createOIDCProvider.ts
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
controlplane/src/core/bufservices/organization/whoAmI.ts (1)
5-6:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix ESLint camelcase error for protobuf-generated import.
The pipeline is failing because
LoginMethod_Typeviolates the camelcase rule. Since this is a protobuf-generated identifier that cannot be renamed, add an ESLint disable comment for this import.🔧 Proposed fix
import { LoginMethod, + // eslint-disable-next-line camelcase LoginMethod_Type, WhoAmIRequest, WhoAmIResponse, } from '`@wundergraph/cosmo-connect/dist/platform/v1/platform_pb`';Alternatively, disable inline at each usage site if the import-level disable doesn't work:
// eslint-disable-next-line camelcase type: LoginMethod_Type.LOGIN_METHOD_TYPE_SSO,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controlplane/src/core/bufservices/organization/whoAmI.ts` around lines 5 - 6, The import of the protobuf-generated identifier LoginMethod_Type violates ESLint's camelcase rule; fix it by adding an inline ESLint disable comment for camelcase on the import line that brings in LoginMethod_Type (i.e., adjust the import that includes LoginMethod and LoginMethod_Type to include "// eslint-disable-next-line camelcase" immediately above it) so the generated identifier is allowed without renaming; if that import-level disable doesn't resolve linting, alternatively add "// eslint-disable-next-line camelcase" at the specific usage sites where LoginMethod_Type is referenced (e.g., where type: LoginMethod_Type.LOGIN_METHOD_TYPE_SSO is used).
🧹 Nitpick comments (1)
controlplane/src/core/bufservices/organization/whoAmI.ts (1)
44-88: 💤 Low valueInconsistent formatting in switch cases.
The switch statement has unusual blank lines before
break;statements (lines 57-58, 67-68, 77-78). This appears to be inconsistent formatting that should be cleaned up for readability.✨ Suggested formatting
case 'sso': { const oidcRepo = new OidcRepository(opts.db); const provider = await oidcRepo.getOidcProviderById({ id: lm.ssoProviderId, organizationId: authContext.organizationId, }); loginMethod = { type: LoginMethod_Type.LOGIN_METHOD_TYPE_SSO, ssoProviderId: lm.ssoProviderId, ssoProviderName: provider?.name ?? '', ssoAlias: lm.alias, }; - break; } case 'password': { loginMethod = { type: LoginMethod_Type.LOGIN_METHOD_TYPE_PASSWORD, ssoProviderId: '', ssoProviderName: '', ssoAlias: '', }; - break; } case 'api-key': { loginMethod = { type: LoginMethod_Type.LOGIN_METHOD_TYPE_API_KEY, ssoProviderId: '', ssoProviderName: '', ssoAlias: '', }; - break; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controlplane/src/core/bufservices/organization/whoAmI.ts` around lines 44 - 88, The switch on lm?.type in whoAmI.ts (cases handling 'sso', 'password', 'api-key', and default that set loginMethod using LoginMethod_Type and OidcRepository/provider) contains stray blank lines immediately before each break; remove those blank lines so each case ends directly with the break statement, ensure consistent spacing/indentation within each case (no extra vertical gaps), and run the project formatter to normalize the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@controlplane/src/core/bufservices/organization/whoAmI.ts`:
- Around line 5-6: The import of the protobuf-generated identifier
LoginMethod_Type violates ESLint's camelcase rule; fix it by adding an inline
ESLint disable comment for camelcase on the import line that brings in
LoginMethod_Type (i.e., adjust the import that includes LoginMethod and
LoginMethod_Type to include "// eslint-disable-next-line camelcase" immediately
above it) so the generated identifier is allowed without renaming; if that
import-level disable doesn't resolve linting, alternatively add "//
eslint-disable-next-line camelcase" at the specific usage sites where
LoginMethod_Type is referenced (e.g., where type:
LoginMethod_Type.LOGIN_METHOD_TYPE_SSO is used).
---
Nitpick comments:
In `@controlplane/src/core/bufservices/organization/whoAmI.ts`:
- Around line 44-88: The switch on lm?.type in whoAmI.ts (cases handling 'sso',
'password', 'api-key', and default that set loginMethod using LoginMethod_Type
and OidcRepository/provider) contains stray blank lines immediately before each
break; remove those blank lines so each case ends directly with the break
statement, ensure consistent spacing/indentation within each case (no extra
vertical gaps), and run the project formatter to normalize the file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3529a86f-cc01-4aa6-934b-722a4f739841
📒 Files selected for processing (4)
controlplane/src/core/bufservices/organization/whoAmI.tsstudio/src/components/user-menu.tsxstudio/src/pages/[organizationSlug]/namespace-sso.tsxstudio/src/pages/[organizationSlug]/settings.tsx
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
controlplane/src/core/services/RBACEvaluator.ts (1)
115-117: 💤 Low valueRedundant IdP gate check in
canCreateContract.
canCreateFederatedGraphalready includes theisAllowedByIdpGatecheck, making the additional call here redundant.♻️ Suggested simplification
canCreateContract(namespace: Namespace) { - return this.canCreateFederatedGraph(namespace) && this.isAllowedByIdpGate(namespace.id); + return this.canCreateFederatedGraph(namespace); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controlplane/src/core/services/RBACEvaluator.ts` around lines 115 - 117, The canCreateContract method redundantly calls isAllowedByIdpGate because canCreateFederatedGraph already performs that check; simplify canCreateContract by returning only the result of canCreateFederatedGraph(namespace) (keep the Namespace parameter) and remove the extra isAllowedByIdpGate(namespace.id) invocation so the logic isn’t duplicated in canCreateContract, referencing the canCreateContract, canCreateFederatedGraph, and isAllowedByIdpGate symbols for location.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@controlplane/src/core/services/RBACEvaluator.ts`:
- Around line 115-117: The canCreateContract method redundantly calls
isAllowedByIdpGate because canCreateFederatedGraph already performs that check;
simplify canCreateContract by returning only the result of
canCreateFederatedGraph(namespace) (keep the Namespace parameter) and remove the
extra isAllowedByIdpGate(namespace.id) invocation so the logic isn’t duplicated
in canCreateContract, referencing the canCreateContract,
canCreateFederatedGraph, and isAllowedByIdpGate symbols for location.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b408b318-6432-4fd9-9105-909569ce3f6e
📒 Files selected for processing (5)
controlplane/src/core/bufservices/federated-graph/moveFederatedGraph.tscontrolplane/src/core/bufservices/organization/whoAmI.tscontrolplane/src/core/bufservices/sso/listOIDCProviders.tscontrolplane/src/core/services/RBACEvaluator.tscontrolplane/src/db/schema.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@controlplane/test/oidc-provider.test.ts`:
- Around line 42-45: Replace the non-deterministic providers[0] selection with a
deterministic lookup: after calling client.listOIDCProviders use
Array.prototype.find to locate the provider by a known unique attribute (e.g.,
provider.name or createdProvider.id) and assert it exists, then pass that
provider.id into client.getOIDCProvider; update the same pattern for the other
occurrences (the blocks that call client.listOIDCProviders and then index
providers) so each test selects by name or by the id returned when creating the
provider rather than using providers[0].
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c54a7ace-fd6a-4e38-9e0c-6a8b77a832aa
📒 Files selected for processing (1)
controlplane/test/oidc-provider.test.ts
comatory
reviewed
May 21, 2026
comatory
reviewed
May 21, 2026
…ss in getSdlBySchemaVersion
comatory
requested changes
May 21, 2026
Contributor
comatory
left a comment
comatory
left a comment
There was a problem hiding this comment.
👍 I have few suggestions but they're all related to readability and structure. Otherwise let me know and I'll test this manually and do another pass at this as soon as I can.
…in IdP gate tests
…r-different' of github.com:wundergraph/cosmo into suvij/eng-9245-controlplane-different-auth-providers-for-different
…rove login method checks
This comment has been minimized.
This comment has been minimized.
…mappings - Added `listNamespaceSSOMappings` function to retrieve SSO mappings for restricted namespaces. - Updated `PlatformService` to include the new API endpoint. - Created `listNamespaceSSOMappings.ts` in the namespace service to handle the logic. - Modified `NamespaceSsoMappingRepository` to support listing mappings with RBAC checks. - Updated tests to validate the new functionality. - Refactored the namespace SSO mapping page to utilize the new API, replacing the previous mapping retrieval logic. - Introduced `NamespaceMappingRows` component for managing SSO mappings in the UI. - Added a new `MultiSelect` component for selecting allowed login methods.
…space SSO mappings - Introduced new LoginMethodType for social logins and updated related types. - Enhanced updateNamespaceSSOMappings to handle Google and GitHub login options. - Updated NamespaceSsoMappingRepository to manage new login methods. - Modified whoAmI and auth controllers to accommodate social login methods. - Updated UI components to reflect changes in login methods and display appropriate labels. - Added tests to ensure correct behavior for social login mappings and access control.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
comatory
reviewed
May 22, 2026
| .where(and(...conditions)) | ||
| .execute(); | ||
|
|
||
| type Entry = { |
Contributor
There was a problem hiding this comment.
I'd just inline it on L124. It's odd creating a standalone type in middle of the function body.
comatory
approved these changes
May 22, 2026
…roviders-for-different
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Security Improvements
UI/UX Enhancements
Chores
Checklist
Open Source AI Manifesto
This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.